feat: add changeMeasurementColor command to update annotation colors …#6092
Open
TalalEmara wants to merge 6 commits into
Open
feat: add changeMeasurementColor command to update annotation colors …#6092TalalEmara wants to merge 6 commits into
TalalEmara wants to merge 6 commits into
Conversation
…via dialog and extend MeasurementService to support color persistence
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a measurement color-change command, persists RGBA color on measurements, and updates Cornerstone annotation styling when measurement colors are added or changed. ChangesMeasurement Color Change Feature
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant UI
participant actions.changeMeasurementColor
participant measurementService
participant colorPickerDialog
participant renderingEngine
UI->>actions.changeMeasurementColor: changeMeasurementColor({ uid })
actions.changeMeasurementColor->>measurementService: getMeasurement(uid)
actions.changeMeasurementColor->>colorPickerDialog: open with current RGBA color
colorPickerDialog-->>actions.changeMeasurementColor: save selected RGBA color
actions.changeMeasurementColor->>measurementService: updateColorMeasurement(uid, colorArray)
actions.changeMeasurementColor->>renderingEngine: render()
PoemPoem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@extensions/cornerstone/src/commandsModule.ts`:
- Around line 695-702: Add a guard check before accessing the color array in the
measurement object. Before the line that assigns color to a constant, verify
that measurement.color exists and is a valid array with the required indices. If
the color is missing or invalid, either provide a default color value or return
early from the function with appropriate handling. This prevents array index
out-of-bounds errors when creating the rgbaColor object with properties r, g, b,
and a.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 83f95af4-aa8e-46c3-817e-6f480e10dabe
📒 Files selected for processing (2)
extensions/cornerstone/src/commandsModule.tsplatform/core/src/services/MeasurementService/MeasurementService.ts
Problem: 1. Unguarded access on `measurement.color` caused a silent crash (TypeError) for older measurements that lacked a color field. 2. The alpha channel was silently dropped during CSS string construction, breaking transparency. 3. Custom colors were lost when viewports were rebuilt (e.g., navigating away and back) because `setAnnotationStyles` was not re-called during annotation reload, causing them to render with the default color despite the custom color remaining in state. Solution: - Added a fallback default array (`?? [255, 255, 0, 255]`) to prevent crashes on measurements without existing color data. - Updated the CSS color string generator to use `rgba()` to properly apply the alpha channel to the canvas. - Synced Cornerstone's style manager with OHIF's `MeasurementService` during all lifecycle events: - `MEASUREMENT_ADDED`: Re-applies saved colors via `setAnnotationStyles` when viewports rebuild. - `RAW_MEASUREMENT_ADDED`: Restores colors when annotations are initially hydrated. - `MEASUREMENT_UPDATED`: Restored the style update block to gracefully sync RGBA colors back to the Cornerstone renderer on changes.
Author
|
Just a friendly ping on this PR. Let me know if there is anything else you need from me to get this reviewed and merged. Thanks!" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…via dialog and extend MeasurementService to support color persistence
Context
This PR addresses a bug where the "Change Color" context menu option for measurements was completely unresponsive because the underlying command was not fully implemented.
Fixes #6059
Changes & Results
changeMeasurementColorcommand toextensions/cornerstone/src/commandsModule.ts.uiDialogServiceto open thecolorPickerDialog.MeasurementServiceto ensure the updatedcolorpersists in the measurement state.Results:
.
Testing
Open any study in the Basic Viewer.
2. Select a measurement tool (e.g., Length or Angle) and draw an annotation on the image.
3. Open the right-side Measurement Panel.
4. Click the three dots (
...) next to the measurement and select Change Color.5. Select a new color from the dialog and click Save.
6. Verify that the measurement on the canvas immediately updates to the new color.
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Summary by CodeRabbit
Greptile Summary
This PR implements the previously unresponsive "Change Color" context menu option for measurements by adding a
changeMeasurementColorcommand, wiring it to a color picker dialog, and extendingMeasurementServiceto persist the chosen color across viewport lifecycle events.changeMeasurementColorcommand (commandsModule.ts): openscolorPickerDialog, converts the picked RGBA to a color array, updatesMeasurementService, callssetAnnotationStyles, and forces a re-render.MeasurementService.ts):addOrUpdatenow carriescolorforward from the previous measurement state; a newupdateColorMeasurementmethod mutates the stored color and broadcastsMEASUREMENT_UPDATED.initMeasurementService.ts): newMEASUREMENT_ADDEDsubscriber and an uncommented block inMEASUREMENT_UPDATEDreapply the stored color viasetAnnotationStyleson every add/update cycle.Confidence Score: 4/5
The color picker dialog works correctly, but the yellow fallback in addOrUpdate will make every annotation drawn without a custom color render yellow instead of the tool's native default.
The hardcoded [255, 255, 0, 255] default stored by addOrUpdate is always truthy, so the if(color) guard in the MEASUREMENT_ADDED subscriber never skips style application for un-colored annotations — all newly drawn annotations are forced to yellow, overriding Cornerstone's per-tool default colors for every user regardless of whether they have ever changed a measurement color.
platform/core/src/services/MeasurementService/MeasurementService.ts — the yellow fallback on line 546 needs to be removed or changed to undefined so the if(color) guards in initMeasurementService work as intended.
Important Files Changed
Reviews (5): Last reviewed commit: "Merge branch 'master' into feat/measurem..." | Re-trigger Greptile